Derive Eq for std::cmp::Ordering, instead of using manual impl. #95017
Conversation
This allows consts of type Ordering to be used in patterns, and (with feature(adt_const_params)) allows using Orderings as const generic parameters.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
Added test in library/core/tests/cmp.rs that ensures that `const`s of type `Ordering`s can be used in patterns.
|
The manual impl appears to have been added in 2013 with this commit ( a0f8540 ). As far as I can tell, |
|
r? @Dylan-DPC |
|
@bors r+ rollup |
|
📌 Commit b13b495 has been approved by |
bors
added
S-waiting-on-bors
…ylan-DPC
Derive Eq for std::cmp::Ordering, instead of using manual impl.
This allows consts of type Ordering to be used in patterns, and with feature(adt_const_params) allows using `Ordering` as a const generic parameter.
Currently, `std::cmp::Ordering` implements `Eq` using a manually written `impl Eq for Ordering {}`, instead of `derive(Eq)`. This means that it does not implement `StructuralEq`.
This commit removes the manually written impl, and adds `derive(Eq)` to `Ordering`, so that it will implement `StructuralEq`.
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#94115 (Let `try_collect` take advantage of `try_fold` overrides) - rust-lang#94295 (Always evaluate all cfg predicate in all() and any()) - rust-lang#94848 (Compare installed browser-ui-test version to the one used in CI) - rust-lang#94993 (Add test for >65535 hashes in lexing raw string) - rust-lang#95017 (Derive Eq for std::cmp::Ordering, instead of using manual impl.) - rust-lang#95058 (Add use of bool::then in sys/unix/process) - rust-lang#95083 (Document that `Option<extern "abi" fn>` discriminant elision applies for any ABI) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
m-ou-se
added
the
T-libs-api
|
This needed a @rust-lang/libs-api FCP, as this was a change to the public stable API. This change has already shipped stably as part of 1.61. Hopefully everyone agrees this change was uncontroversial. |
|
@m-ou-se What was the public API change here? Is my mental model that |
|
This adds an implementation of |
|
Oh wow, okay, that's subtle. Hopefully we can figure out how to make such changes more explicit in the future. But I think this specific change looks okay at least. |
fn main() {
const ORDERING: std::cmp::Ordering = std::cmp::Ordering::Less;
if let ORDERING = ORDERING {}
}$ cargo +1.61.0 check
Checking repro v0.0.0
Finished dev [unoptimized + debuginfo] target(s) in 0.66s
$ cargo +1.60.0 check
Checking repro v0.0.0
error: to use a constant of type `std::cmp::Ordering` in a pattern, `std::cmp::Ordering` must be annotated with `#[derive(PartialEq, Eq)]`
--> src/main.rs:3:12
|
3 | if let ORDERING = ORDERING {}
| ^^^^^^^^
warning: irrefutable `if let` pattern
--> src/main.rs:3:8
|
3 | if let ORDERING = ORDERING {}
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(irrefutable_let_patterns)]` on by default
= note: this pattern will always match, so the `if let` is useless
= help: consider replacing the `if let` with a `let`
error: could not compile `repro` due to previous error; 1 warning emitted |
This allows consts of type Ordering to be used in patterns, and with feature(adt_const_params) allows using
Orderingas a const generic parameter.Currently,
std::cmp::OrderingimplementsEqusing a manually writtenimpl Eq for Ordering {}, instead ofderive(Eq). This means that it does not implementStructuralEq.This commit removes the manually written impl, and adds
derive(Eq)toOrdering, so that it will implementStructuralEq.